Add publish worker #115
Add publish worker #115
Conversation
|
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @richardhuaaa and the rest of your teammates on |
richardhuaaa
force-pushed
the
08-20-add_db_subscription
branch
from
647b702 to
92f5ac8
Compare
richardhuaaa
force-pushed
the
08-20-add_publish_worker
branch
from
b994784 to
cb09162
Compare
richardhuaaa
force-pushed
the
08-20-add_db_subscription
branch
from
92f5ac8 to
6ce44be
Compare
richardhuaaa
force-pushed
the
08-20-add_publish_worker
branch
2 times, most recently
from
177ea52 to
16b2940
Compare
richardhuaaa
force-pushed
the
08-20-add_db_subscription
branch
from
6ce44be to
a100969
Compare
richardhuaaa
force-pushed
the
08-20-add_publish_worker
branch
from
16b2940 to
d27f00a
Compare
richardhuaaa
force-pushed
the
08-20-add_db_subscription
branch
2 times, most recently
from
4467348 to
db44f65
Compare
richardhuaaa
force-pushed
the
08-20-add_publish_worker
branch
3 times, most recently
from
2b75efb to
0752a9b
Compare
| ) | ||
| if err != nil { | ||
| logger.Error("Failed to insert gateway envelope", zap.Error(err)) | ||
| return false |
There was a problem hiding this comment.
There is a risk here that we infinitely retry on this step or one of the steps above in an unanticipated error case. I think that we should be okay here but a second pair of eyes can't hurt
There was a problem hiding this comment.
I don't know why the proto marshaling would fail, but if it did we would be in an endless loop of retries here.
Would probably have to be a maliciously crafted payload
There was a problem hiding this comment.
From my research, it seems like it fails if some expected fields of the proto are not set. In this case, the proto is OriginatorEnvelope with two fields that we are setting here, so I expect that we should be able to prevent this error happening.
I still think we should catch the error just in case though, firstly so that we can detect it, secondly so that we don't screw up payload ordering if there is an unanticipated error type that is super common.
richardhuaaa
force-pushed
the
08-20-add_publish_worker
branch
from
0752a9b to
a8cef43
Compare
gateway_envelopestablestaged_originated_envelopestable - we extract this during the API call, so that the publish worker doesn't need to do any additional unmarshaling or validation.Would particularly love feedback on the error handling in the worker, and if there's any test cases I should add to
service_test.go.